fix: Add dedicated TPM license move option#1909
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe onboarding modal no longer treats the login page specially and unauthenticated mock support was removed. A "Move License to TPM" flow was added: UI button triggers a new Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Registration UI
participant Account as Account Store
participant ServerStore as Server Store (serverReplacePayload)
participant Backend as send()/Backend
UI->>Account: click "Move License to TPM"
Account->>ServerStore: read serverReplacePayload
ServerStore-->>Account: serverReplacePayload (with replaced guid)
Account->>Backend: send ACCOUNT_CALLBACK {type: "replace", payload: serverReplacePayload}
Backend-->>Account: ack
Account-->>UI: update state / close UI
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
web/__test__/store/server.test.ts (1)
671-696: Add tests for the remaining GUID-selection branches.These tests cover TPM-available and TPM-equals-flash paths, but Line 211 also branches on
guid.startsWith('03-')and missingtpmGuid. Add one case for each to guard regressions in TPM-boot and no-TPM states.Suggested test additions
+ it('should create serverReplacePayload with flash guid when booted from TPM guid', () => { + const store = getStore(); + + store.setServer({ + flashGuid: '058F-6387-0000-0000F1F1E1C6', + guid: '03-V35H8S0L1QHK1SBG1XHXJNH7', + state: 'PRO' as ServerState, + tpmGuid: '03-V35H8S0L1QHK1SBG1XHXJNH7', + }); + + expect(store.serverReplacePayload.guid).toBe('058F-6387-0000-0000F1F1E1C6'); + }); + + it('should create serverReplacePayload with flash guid when TPM guid is missing', () => { + const store = getStore(); + + store.setServer({ + flashGuid: '058F-6387-0000-0000F1F1E1C6', + guid: '058F-6387-0000-0000F1F1E1C6', + state: 'PRO' as ServerState, + tpmGuid: undefined, + }); + + expect(store.serverReplacePayload.guid).toBe('058F-6387-0000-0000F1F1E1C6'); + });Based on learnings: Applies to /test/store//*.ts : Test Pinia store getter dependencies are properly mocked.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/__test__/store/server.test.ts` around lines 671 - 696, Add two unit tests to cover the remaining GUID-selection branches in the store: (1) a case where the server.guid startsWith('03-') and tpmGuid is undefined — call getStore(), then store.setServer({ guid: '03-XXXX...', flashGuid: '058F-...', state: 'PRO' as ServerState }) and assert store.serverReplacePayload.guid matches the server.guid (the '03-' GUID) to cover the TPM-boot branch; (2) a case where tpmGuid is missing (undefined) and guid does not start with '03-' — call getStore(), then store.setServer({ guid: '058F-...', flashGuid: '058F-...', state: 'PRO' as ServerState }) and assert store.serverReplacePayload.guid equals the flashGuid to cover the no-TPM branch; reuse the existing pattern around getStore(), store.setServer(...) and assertions for serverReplacePayload.guid.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/src/store/server.ts`:
- Around line 137-145: The computed replaceFlashGuid currently falls back to
flashGuid.value which can be missing/stale; change its logic to use guid.value
as the default and only substitute tpmGuid.value when bootDeviceType.value ===
'flash' && hasDistinctTpmGuid.value, i.e. return tpmGuid.value in that
distinct-TPM case otherwise return guid.value || undefined; update any related
usage in serverReplacePayload and the showTpmTransferInfo logic in
Registration.standalone.vue to use the same fallback (guid over flashGuid) and
only swap to tpmGuid for the distinct TPM path to avoid overwriting a valid
current guid with undefined.
---
Nitpick comments:
In `@web/__test__/store/server.test.ts`:
- Around line 671-696: Add two unit tests to cover the remaining GUID-selection
branches in the store: (1) a case where the server.guid startsWith('03-') and
tpmGuid is undefined — call getStore(), then store.setServer({ guid:
'03-XXXX...', flashGuid: '058F-...', state: 'PRO' as ServerState }) and assert
store.serverReplacePayload.guid matches the server.guid (the '03-' GUID) to
cover the TPM-boot branch; (2) a case where tpmGuid is missing (undefined) and
guid does not start with '03-' — call getStore(), then store.setServer({ guid:
'058F-...', flashGuid: '058F-...', state: 'PRO' as ServerState }) and assert
store.serverReplacePayload.guid equals the flashGuid to cover the no-TPM branch;
reuse the existing pattern around getStore(), store.setServer(...) and
assertions for serverReplacePayload.guid.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 11d971e9-4947-4426-94c1-a05033f9bcd0
📒 Files selected for processing (11)
web/__test__/components/Onboarding/OnboardingModal.test.tsweb/__test__/components/Registration.test.tsweb/__test__/store/account.test.tsweb/__test__/store/server.test.tsweb/src/components/Onboarding/OnboardingModal.vueweb/src/components/Onboarding/standalone/OnboardingAdminPanel.standalone.vueweb/src/components/Onboarding/store/onboardingStatus.tsweb/src/components/Registration.standalone.vueweb/src/locales/en.jsonweb/src/store/account.tsweb/src/store/server.ts
💤 Files with no reviewable changes (2)
- web/src/components/Onboarding/standalone/OnboardingAdminPanel.standalone.vue
- web/test/components/Onboarding/OnboardingModal.test.ts
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1909 +/- ##
==========================================
- Coverage 50.97% 50.94% -0.03%
==========================================
Files 1023 1023
Lines 70701 70542 -159
Branches 7703 7681 -22
==========================================
- Hits 36038 35941 -97
+ Misses 34539 34477 -62
Partials 124 124 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary
Move License to TPMaction on the registration page when a distinct TPM GUID is availableReplace Keyflow unchanged while adding a TPM-specific account callback payloadTesting
Notes
Summary by CodeRabbit
Bug Fixes
New Features
UI/UX